From 60aadc583de0ff42b6b8178c7c55f3b19c9df0c5 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 2 Jun 2017 17:13:52 -0700 Subject: [PATCH] Remove DependencyInner as a public API No need for it to be exposed any more, let's just use `Rc::make_mut` judiciously. --- src/cargo/core/dependency.rs | 315 +++++++++++++----------------- src/cargo/core/mod.rs | 2 +- src/cargo/core/registry.rs | 8 +- src/cargo/ops/resolve.rs | 4 +- src/cargo/sources/registry/mod.rs | 6 +- src/cargo/util/toml.rs | 10 +- 6 files changed, 154 insertions(+), 191 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 4b363e6a7..c748ee9c5 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -14,12 +14,12 @@ use util::errors::{CargoResult, CargoResultExt, CargoError}; /// Cheap to copy. #[derive(PartialEq, Clone, Debug)] pub struct Dependency { - inner: Rc, + inner: Rc, } /// The data underlying a Dependency. #[derive(PartialEq, Clone, Debug)] -pub struct DependencyInner { +struct Inner { name: String, source_id: SourceId, req: VersionReq, @@ -79,6 +79,40 @@ pub enum Kind { Build, } +fn parse_req_with_deprecated(req: &str, + extra: Option<(&PackageId, &Config)>) + -> CargoResult { + match VersionReq::parse(req) { + Err(e) => { + let (inside, config) = match extra { + Some(pair) => pair, + None => return Err(e.into()), + }; + match e { + ReqParseError::DeprecatedVersionRequirement(requirement) => { + let msg = format!("\ +parsed version requirement `{}` is no longer valid + +Previous versions of Cargo accepted this malformed requirement, +but it is being deprecated. This was found when parsing the manifest +of {} {}, and the correct version requirement is `{}`. + +This will soon become a hard error, so it's either recommended to +update to a fixed version or contact the upstream maintainer about +this warning. +", +req, inside.name(), inside.version(), requirement); + config.shell().warn(&msg)?; + + Ok(requirement) + } + e => Err(e.into()), + } + }, + Ok(v) => Ok(v), + } +} + impl ser::Serialize for Kind { fn serialize(&self, s: S) -> Result where S: ser::Serializer, @@ -91,252 +125,178 @@ impl ser::Serialize for Kind { } } -impl DependencyInner { +impl Dependency { /// Attempt to create a `Dependency` from an entry in the manifest. - /// - /// The `deprecated_extra` information is set to `Some` when this method - /// should *also* parse historically deprecated semver requirements. This - /// information allows providing diagnostic information about what's going - /// on. - /// - /// If set to `None`, then historically deprecated semver requirements will - /// all be rejected. pub fn parse(name: &str, version: Option<&str>, source_id: &SourceId, - deprecated_extra: Option<(&PackageId, &Config)>) - -> CargoResult { + inside: &PackageId, + config: &Config) -> CargoResult { + let arg = Some((inside, config)); let (specified_req, version_req) = match version { - Some(v) => (true, DependencyInner::parse_with_deprecated(v, deprecated_extra)?), + Some(v) => (true, parse_req_with_deprecated(v, arg)?), None => (false, VersionReq::any()) }; - Ok(DependencyInner { - only_match_name: false, - req: version_req, - specified_req: specified_req, - .. DependencyInner::new_override(name, source_id) - }) + let mut ret = Dependency::new_override(name, source_id); + { + let ptr = Rc::make_mut(&mut ret.inner); + ptr.only_match_name = false; + ptr.req = version_req; + ptr.specified_req = specified_req; + } + Ok(ret) } - fn parse_with_deprecated(req: &str, - extra: Option<(&PackageId, &Config)>) - -> CargoResult { - match VersionReq::parse(req) { - Err(e) => { - let (inside, config) = match extra { - Some(pair) => pair, - None => return Err(e.into()), - }; - match e { - ReqParseError::DeprecatedVersionRequirement(requirement) => { - let msg = format!("\ -parsed version requirement `{}` is no longer valid - -Previous versions of Cargo accepted this malformed requirement, -but it is being deprecated. This was found when parsing the manifest -of {} {}, and the correct version requirement is `{}`. - -This will soon become a hard error, so it's either recommended to -update to a fixed version or contact the upstream maintainer about -this warning. -", - req, inside.name(), inside.version(), requirement); - config.shell().warn(&msg)?; + /// Attempt to create a `Dependency` from an entry in the manifest. + pub fn parse_no_deprecated(name: &str, + version: Option<&str>, + source_id: &SourceId) -> CargoResult { + let (specified_req, version_req) = match version { + Some(v) => (true, parse_req_with_deprecated(v, None)?), + None => (false, VersionReq::any()) + }; - Ok(requirement) - } - e => Err(e.into()), - } - }, - Ok(v) => Ok(v), + let mut ret = Dependency::new_override(name, source_id); + { + let ptr = Rc::make_mut(&mut ret.inner); + ptr.only_match_name = false; + ptr.req = version_req; + ptr.specified_req = specified_req; } + Ok(ret) } - pub fn new_override(name: &str, source_id: &SourceId) -> DependencyInner { - DependencyInner { - name: name.to_string(), - source_id: source_id.clone(), - req: VersionReq::any(), - kind: Kind::Normal, - only_match_name: true, - optional: false, - features: Vec::new(), - default_features: true, - specified_req: false, - platform: None, + pub fn new_override(name: &str, source_id: &SourceId) -> Dependency { + Dependency { + inner: Rc::new(Inner { + name: name.to_string(), + source_id: source_id.clone(), + req: VersionReq::any(), + kind: Kind::Normal, + only_match_name: true, + optional: false, + features: Vec::new(), + default_features: true, + specified_req: false, + platform: None, + }), } } - pub fn version_req(&self) -> &VersionReq { &self.req } - pub fn name(&self) -> &str { &self.name } - pub fn source_id(&self) -> &SourceId { &self.source_id } - pub fn kind(&self) -> Kind { self.kind } - pub fn specified_req(&self) -> bool { self.specified_req } + pub fn version_req(&self) -> &VersionReq { + &self.inner.req + } - /// If none, this dependency must be built for all platforms. - /// If some, it must only be built for matching platforms. + pub fn name(&self) -> &str { + &self.inner.name + } + + pub fn source_id(&self) -> &SourceId { + &self.inner.source_id + } + + pub fn kind(&self) -> Kind { + self.inner.kind + } + + pub fn specified_req(&self) -> bool { + self.inner.specified_req + } + + /// If none, this dependencies must be built for all platforms. + /// If some, it must only be built for the specified platform. pub fn platform(&self) -> Option<&Platform> { - self.platform.as_ref() + self.inner.platform.as_ref() } - pub fn set_kind(&mut self, kind: Kind) -> &mut DependencyInner { - self.kind = kind; + pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency { + Rc::make_mut(&mut self.inner).kind = kind; self } /// Sets the list of features requested for the package. - pub fn set_features(&mut self, features: Vec) -> &mut DependencyInner { - self.features = features; + pub fn set_features(&mut self, features: Vec) -> &mut Dependency { + Rc::make_mut(&mut self.inner).features = features; self } /// Sets whether the dependency requests default features of the package. - pub fn set_default_features(&mut self, default_features: bool) -> &mut DependencyInner { - self.default_features = default_features; + pub fn set_default_features(&mut self, default_features: bool) -> &mut Dependency { + Rc::make_mut(&mut self.inner).default_features = default_features; self } /// Sets whether the dependency is optional. - pub fn set_optional(&mut self, optional: bool) -> &mut DependencyInner { - self.optional = optional; + pub fn set_optional(&mut self, optional: bool) -> &mut Dependency { + Rc::make_mut(&mut self.inner).optional = optional; self } /// Set the source id for this dependency - pub fn set_source_id(&mut self, id: SourceId) -> &mut DependencyInner { - self.source_id = id; + pub fn set_source_id(&mut self, id: SourceId) -> &mut Dependency { + Rc::make_mut(&mut self.inner).source_id = id; self } /// Set the version requirement for this dependency - pub fn set_version_req(&mut self, req: VersionReq) -> &mut DependencyInner { - self.req = req; + pub fn set_version_req(&mut self, req: VersionReq) -> &mut Dependency { + Rc::make_mut(&mut self.inner).req = req; self } - pub fn set_platform(&mut self, platform: Option) - -> &mut DependencyInner { - self.platform = platform; + pub fn set_platform(&mut self, platform: Option) -> &mut Dependency { + Rc::make_mut(&mut self.inner).platform = platform; self } /// Lock this dependency to depending on the specified package id - pub fn lock_to(&mut self, id: &PackageId) -> &mut DependencyInner { - assert_eq!(self.source_id, *id.source_id()); - assert!(self.req.matches(id.version())); + pub fn lock_to(&mut self, id: &PackageId) -> &mut Dependency { + assert_eq!(self.inner.source_id, *id.source_id()); + assert!(self.inner.req.matches(id.version())); self.set_version_req(VersionReq::exact(id.version())) .set_source_id(id.source_id().clone()) } + /// Returns false if the dependency is only used to build the local package. pub fn is_transitive(&self) -> bool { - match self.kind { + match self.inner.kind { Kind::Normal | Kind::Build => true, Kind::Development => false, } } - pub fn is_build(&self) -> bool { - match self.kind { Kind::Build => true, _ => false } - } - pub fn is_optional(&self) -> bool { self.optional } - /// Returns true if the default features of the dependency are requested. - pub fn uses_default_features(&self) -> bool { self.default_features } - /// Returns the list of features that are requested by the dependency. - pub fn features(&self) -> &[String] { &self.features } - - /// Returns true if the package (`sum`) can fulfill this dependency request. - pub fn matches(&self, sum: &Summary) -> bool { - self.matches_id(sum.package_id()) - } - - /// Returns true if the package (`id`) can fulfill this dependency request. - pub fn matches_id(&self, id: &PackageId) -> bool { - self.name == id.name() && - (self.only_match_name || (self.req.matches(id.version()) && - &self.source_id == id.source_id())) - } - - pub fn into_dependency(self) -> Dependency { - Dependency {inner: Rc::new(self)} - } -} - -impl Dependency { - /// Attempt to create a `Dependency` from an entry in the manifest. - pub fn parse(name: &str, - version: Option<&str>, - source_id: &SourceId, - inside: &PackageId, - config: &Config) -> CargoResult { - let arg = Some((inside, config)); - DependencyInner::parse(name, version, source_id, arg).map(|di| { - di.into_dependency() - }) - } - - /// Attempt to create a `Dependency` from an entry in the manifest. - pub fn parse_no_deprecated(name: &str, - version: Option<&str>, - source_id: &SourceId) -> CargoResult { - DependencyInner::parse(name, version, source_id, None).map(|di| { - di.into_dependency() - }) - } - - pub fn new_override(name: &str, source_id: &SourceId) -> Dependency { - DependencyInner::new_override(name, source_id).into_dependency() - } - - pub fn clone_inner(&self) -> DependencyInner { (*self.inner).clone() } - pub fn version_req(&self) -> &VersionReq { self.inner.version_req() } - pub fn name(&self) -> &str { self.inner.name() } - pub fn source_id(&self) -> &SourceId { self.inner.source_id() } - pub fn kind(&self) -> Kind { self.inner.kind() } - pub fn specified_req(&self) -> bool { self.inner.specified_req() } - - /// If none, this dependencies must be built for all platforms. - /// If some, it must only be built for the specified platform. - pub fn platform(&self) -> Option<&Platform> { - self.inner.platform() - } - - /// Lock this dependency to depending on the specified package id - pub fn lock_to(mut self, id: &PackageId) -> Dependency { - Rc::make_mut(&mut self.inner).lock_to(id); - self - } - - /// Set the version requirement for this dependency - pub fn set_version_req(&mut self, req: VersionReq) -> &mut Dependency { - Rc::make_mut(&mut self.inner).set_version_req(req); - self + pub fn is_build(&self) -> bool { + match self.inner.kind { + Kind::Build => true, + _ => false, + } } - pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency { - Rc::make_mut(&mut self.inner).set_kind(kind); - self + pub fn is_optional(&self) -> bool { + self.inner.optional } - /// Returns false if the dependency is only used to build the local package. - pub fn is_transitive(&self) -> bool { self.inner.is_transitive() } - pub fn is_build(&self) -> bool { self.inner.is_build() } - pub fn is_optional(&self) -> bool { self.inner.is_optional() } - /// Returns true if the default features of the dependency are requested. pub fn uses_default_features(&self) -> bool { - self.inner.uses_default_features() + self.inner.default_features } /// Returns the list of features that are requested by the dependency. - pub fn features(&self) -> &[String] { self.inner.features() } + pub fn features(&self) -> &[String] { + &self.inner.features + } /// Returns true if the package (`sum`) can fulfill this dependency request. - pub fn matches(&self, sum: &Summary) -> bool { self.inner.matches(sum) } + pub fn matches(&self, sum: &Summary) -> bool { + self.matches_id(sum.package_id()) + } /// Returns true if the package (`id`) can fulfill this dependency request. pub fn matches_id(&self, id: &PackageId) -> bool { - self.inner.matches_id(id) + self.inner.name == id.name() && + (self.inner.only_match_name || (self.inner.req.matches(id.version()) && + &self.inner.source_id == id.source_id())) } pub fn map_source(mut self, to_replace: &SourceId, replace_with: &SourceId) @@ -344,8 +304,7 @@ impl Dependency { if self.source_id() != to_replace { self } else { - Rc::make_mut(&mut self.inner) - .set_source_id(replace_with.clone()); + self.set_source_id(replace_with.clone()); self } } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index e663dcdf2..2c966f681 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,4 +1,4 @@ -pub use self::dependency::{Dependency, DependencyInner}; +pub use self::dependency::Dependency; pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles}; pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::package::{Package, PackageSet}; diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 6c7962049..7c5292604 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -259,7 +259,7 @@ impl<'cfg> PackageRegistry<'cfg> { Some(&(ref precise, _)) => summary.override_id(precise.clone()), None => summary, }; - summary.map_dependencies(|dep| { + summary.map_dependencies(|mut dep| { trace!("\t{}/{}/{}", dep.name(), dep.version_req(), dep.source_id()); @@ -286,7 +286,8 @@ impl<'cfg> PackageRegistry<'cfg> { let locked = locked_deps.iter().find(|id| dep.matches_id(id)); if let Some(locked) = locked { trace!("\tfirst hit on {}", locked); - return dep.lock_to(locked) + dep.lock_to(locked); + return dep } } @@ -301,7 +302,8 @@ impl<'cfg> PackageRegistry<'cfg> { match v { Some(&(ref id, _)) => { trace!("\tsecond hit on {}", id); - dep.lock_to(id) + dep.lock_to(id); + return dep } None => { trace!("\tremaining unlocked"); diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 8741c7425..8a84071b4 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -217,7 +217,9 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry, if spec.matches(key) && dep.matches_id(val) && keep(&val, to_avoid, &to_avoid_sources) { - return (spec.clone(), dep.clone().lock_to(val)) + let mut dep = dep.clone(); + dep.lock_to(val); + return (spec.clone(), dep) } } (spec.clone(), dep.clone()) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 84f801ed8..beb768759 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -170,7 +170,7 @@ use serde::de; use tar::Archive; use core::{Source, SourceId, PackageId, Package, Summary, Registry}; -use core::dependency::{Dependency, DependencyInner, Kind}; +use core::dependency::{Dependency, Kind}; use sources::PathSource; use util::{CargoResult, Config, internal, FileLock, Filesystem}; use util::errors::CargoResultExt; @@ -459,7 +459,7 @@ fn parse_registry_dependency(dep: RegistryDependency) } = dep; let mut dep = DEFAULT_ID.with(|id| { - DependencyInner::parse(&name, Some(&req), id, None) + Dependency::parse_no_deprecated(&name, Some(&req), id) })?; let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") { "dev" => Kind::Development, @@ -484,5 +484,5 @@ fn parse_registry_dependency(dep: RegistryDependency) .set_features(features) .set_platform(platform) .set_kind(kind); - Ok(dep.into_dependency()) + Ok(dep) } diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index b665d322d..57fe72abb 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -12,7 +12,7 @@ use serde::de::{self, Deserialize}; use serde_ignored; use core::{SourceId, Profiles, PackageIdSpec, GitReference, WorkspaceConfig}; -use core::{Summary, Manifest, Target, Dependency, DependencyInner, PackageId}; +use core::{Summary, Manifest, Target, Dependency, PackageId}; use core::{EitherManifest, VirtualManifest}; use core::dependency::{Kind, Platform}; use core::manifest::{LibKind, Profile, ManifestMetadata}; @@ -1111,10 +1111,10 @@ impl TomlDependency { let version = details.version.as_ref().map(|v| &v[..]); let mut dep = match cx.pkgid { Some(id) => { - DependencyInner::parse(name, version, &new_source_id, - Some((id, cx.config)))? + Dependency::parse(name, version, &new_source_id, + id, cx.config)? } - None => DependencyInner::parse(name, version, &new_source_id, None)?, + None => Dependency::parse_no_deprecated(name, version, &new_source_id)?, }; dep.set_features(details.features.unwrap_or(Vec::new())) .set_default_features(details.default_features @@ -1125,7 +1125,7 @@ impl TomlDependency { if let Some(kind) = kind { dep.set_kind(kind); } - Ok(dep.into_dependency()) + Ok(dep) } } -- 2.30.2